New components by smnh · Pull Request #1 · stackbit/stackbit-components

smnh · 2021-07-21T16:51:53Z

Task:
https://www.notion.so/stackbit/Implement-2-themes-and-5-6-components-87c240fb9bb141689530219e6d86673b

netlify · 2021-07-21T16:51:59Z

✔️ Deploy Preview for stackbit-components ready!

🔨 Explore the source changes: 84cd131

🔍 Inspect the deploy log: https://app.netlify.com/sites/stackbit-components/deploys/6109d0a779b42200079dfb8f

😎 Browse the preview: https://deploy-preview-1--stackbit-components.netlify.app

smnh · 2021-07-21T17:10:44Z

@TomasBankauskas good start!
Here is some feedback.

The hero section have 4 variants, but they all look the same. For example the variant-c and variant-d has the exact same code. The difference is that one has image and the other has video. For these differences we want to use atomic components rather having different variants. Otherwise, the component will be bloated with lots of fields not related to one another like imageUrl, imageAlt, videoUrl, etc. Variants should really change the whole appearance of the section not of specific item in the section.

For example, to have a hero section with image or video, you can have a field feature that can be an image or video model. These models in turn will have URL for image or video, alt text, and anything else these specific components need.

fields:
  - type: string
    name: title
  - type: string
    name: subtitle
  - type: model
    name: feature
    models: [image, video]

The hero section has both variant and mediaPosition. This kinda kills the purpose of variants. You should have either first or the second.

Consider renaming style to colors

smnh

@TomasBankauskas good work! 👍
I've commented here on some thing that we need to improve, but you can already merge it.

smnh · 2021-08-01T11:54:18Z

+        case 'variant-e':
+            return HeroNoFeature(props);


@TomasBankauskas I think there is no need in variant-e. Because in any variant if the feature object is not set, then it will be the same as HeroNoFeature.
You can check here if the feature is not set, then you can render HeroNoFeature and remove variant-e fomt he model

smnh · 2021-08-01T11:57:02Z

+    if (!feature) {
+        return null;
+    }


@TomasBankauskas returning null here is too late. Because you will get an empty div:

<div className="mt-8 lg:mt-12"> </div>

smnh · 2021-08-01T12:00:44Z

+                'max-w-screen-xl': width === 'wide',
+                'max-w-screen-lg': width === 'narrow',
+                'min-h-screen flex flex-col justify-center': height === 'viewport',
+                'text-center': alignHoriz === 'center'


Will it be ok if we add alignment "right"?

smnh · 2021-08-01T13:32:15Z

+            {props.badge && <Badge label={props.badge} />}
+            {props.title && (
+                <h1 className="font-medium font-sans text-4xl tracking-tight sm:text-5xl mb-6">
+                    <ReactMarkdown allowedElements={['br', 'span', 'strong']} unwrapDisallowed={true} components={components}>


The allowedElements might be too limited, what about italics (*italics*) or links ([link](https://...))?
Let's use markdown-to-jsx. There is also an option forceInline which will ensure that it will not try to wrap it with block elements like <p> or <h1>

P.S.:
This can wait for next iteration

smnh · 2021-08-01T14:27:37Z

+                'mx-auto': width !== 'full',
+                'max-w-screen-xl': width === 'wide',
+                'max-w-screen-lg': width === 'narrow',


@TomasBankauskas this code snippet is used in 6 places.
Is it possible to make it a utility function, or even better, remap it to a new utility style like section-w-full, section-w-wide, section-w-narrow?
To make it easier for developers to update style via tailwind.config.js rather cloning and customizing all components.

smnh · 2021-08-01T14:29:39Z

+                'mx-auto': width !== 'full',
+                'max-w-screen-xl': width === 'wide',
+                'max-w-screen-lg': width === 'narrow',
+                'min-h-screen flex flex-col justify-center': height === 'viewport',


Same for this one, can we create a utility class section-h-viewport? This way developers will be able to customize all section heights without customizing all sections.

smnh · 2021-08-01T14:38:00Z

+            <div
+                className="w-full lg:inset-y-0 lg:w-1/2 lg:max-w-full lg:absolute right-0 lg:pl-4">
+                <img
+                    src={props.imageUrl}
+                    className="w-screen max-w-none ml-1/2 transform -translate-x-1/2 object-cover lg:h-full lg:ml-0 lg:transform-none lg:w-full lg:max-w-full"
+                    alt={props.imageAltText}
+                />
+            </div>


Consider using ImageBlock here. Even if it is the only possible type for this section.

smnh · 2021-08-01T14:45:16Z

+    switch (feature.type) {
+        case 'image_block':
+            return <ImageBlock {...feature} className={classNames({ 'mx-auto': alignHoriz === 'center' })} />;
+        case 'video_block':
+            return <VideoBlock {...feature} />;
+    }


Unlike the variants which are constant, types that can be embedded into Hero section are dynamic.

Component Categories:

fields: - type: model name: feature label: Feature categories: [hero_block]

That means that feature field can container any model, not only image_block or video_block.
Therefore, the code should be similar to how it works in Advanced layout:

const Component = components[sectionType]; return <Component {...feature} />;

Note: the models YAML don't support categories yet.

smnh · 2021-08-01T14:51:04Z

 export default function Button({ label, url, icon, alt, className }) {
+    const iconMap = {
+        arrowRight: ArrowRight,
+        cart: Cart
+    };


Don't we support button primary anymore?
How would user be able to change button from primary to secondary without changing the style of the whole section?

smnh · 2021-08-01T14:53:42Z

+    switch (field.inputType) {
+        case 'checkbox':
+            return (


Here, the form controls should be separates into separate components, each with their own type.
And the form would have a list of fields models categorized as "form-field".
Then developer will be able to create new types of "form-fields":

const Component = components[field.type]; return <Component {...field} />;

smnh · 2021-08-01T16:07:09Z

+        '.colors-a': {
+          backgroundColor: theme('colors.base-50'),
+          color: theme('colors.base-900'),
+          'input,textarea,select,hr': {
+            borderColor: theme('colors.base-900')
+          },
+          '::placeholder': {
+            color: theme('colors.base-900')
+          },
+          '.sb-btn': {
+            backgroundColor: theme('colors.primary'),
+            color: theme('colors.base-900'),
+            '&:hover': {
+              backgroundColor: theme('colors.primary-variant')
+            },
+          },
+          '.sb-card': {
+            backgroundColor: theme('colors.secondary')
+          }
+        },


@TomasBankauskas is there a way to define flat object with colors and apply them to components like in DaisyUI?
In DaisyUI, they use it for themes, so each theme has a flat object with different set of colors that is applied everywhere else.
https://github.com/saadeghi/daisyui/blob/master/src/colors/themes.js#L2-L23
Then buttons assigned base text colors and background colors:
https://github.com/saadeghi/daisyui/blob/master/src/components/styled/button.css#L26-L56

- added dynamic/imports - moved models to root folder

TomasBankauskas added 11 commits July 14, 2021 16:05

added new components

08d2250

updated hero

1f304cf

archived old components

5487763

added width and height to sections

5f10814

updated hero-section.stories.js

2c9de81

removed kitwind.js

463dfeb

added style options

8e50282

updated styles

1c6eb2a

added cta component

79baac9

added Babel

200d0e7

formatted code

664ae73

smnh marked this pull request as draft July 21, 2021 16:52

TomasBankauskas added 9 commits July 25, 2021 19:42

updated components

735f235

few improvements

9367455

few improvements

496d7ad

updated style switching in components

2ce8a4c

added models

4991238

added new tailwind configs

b98747f

adjusted style in Tailwind configs

c7aa9b1

updated btns CSS

d30bd74

added hover state to btns

c82cda6

smnh commented Aug 1, 2021

View reviewed changes

TomasBankauskas and others added 3 commits August 2, 2021 16:58

adjusted components

befab02

add index to components folder

2edd345

improvements

84cd131

- added dynamic/imports - moved models to root folder

smnh marked this pull request as ready for review August 3, 2021 23:28

smnh merged commit 2c8d43c into main Aug 3, 2021

smnh deleted the new-components branch August 3, 2021 23:28

Conversation

smnh commented Jul 21, 2021

Uh oh!

netlify bot commented Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smnh commented Jul 21, 2021

Uh oh!

smnh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smnh Aug 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Jul 21, 2021 •

edited

Loading

smnh left a comment •

edited

Loading

smnh Aug 1, 2021 •

edited

Loading